-
Notifications
You must be signed in to change notification settings - Fork 150
Support for temporal types #341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This commit makes driver able to receive temporal types as query results and send them as query parameters. All types are represented by custom date-time classes because native `Date` type is quite limited. It can't represent needed millisecond range and does not have support for things like timezone. All temporal types expose understandable properties. For example `CypherDate` has `year`, `month` and `day` properties. Driver uses code adapted from https://github.com/ThreeTen/threetenbp to perform the conversions. Main difference is that driver operates on `Integer` objects and not native JavaScript numbers.
It returns ISO strings for every temporal type except DateTime with time zone ID. Later is formatted to an ISO date-time and time zone ID is appended in square brackets. Ideally driver would also include offset but there is no way to get it without a third-party library. Offset might be included in future.
And use top-level import in temporal tests.
TCK rewrites ids of nodes and relationships in received paths with fake ids that start from zero. This is done to easily compare predefined path from feature file with received path. It makes entity ids predictable. This commit removes incorrect assertion during path rewriting. Previously this assertion did not fail because `Integer#notEquals()` always returned a falsy value due to a bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Only one very small comment on variable naming.
src/v1/internal/packstream-v2.js
Outdated
} | ||
|
||
function packLocalTime(value, packer, onError) { | ||
const totalNanos = cypherLocalTimeToNanoOfDay(value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe name this as nanoOfDay
instead of totalNanos
?
src/v1/internal/packstream-v2.js
Outdated
* @param {function} onError the error callback. | ||
*/ | ||
function packTime(value, packer, onError) { | ||
const totalNanos = cypherLocalTimeToNanoOfDay(value.localTime); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here?
They will mostly be used with `neo4j.` namespace so "Cypher" prefix seems unnecessary. Types can also be imported with an alias.
This PR makes driver able to receive temporal types as query results and send them as query parameters. All types are represented by custom date-time classes because native
Date
type is quite limited. It can't represent needed millisecond range and does not have support for things like timezone.New types are:
Duration
- representsDuration
Cypher typeLocalTime
- representsLocalTime
Cypher typeTime
- representsTime
Cypher typeDate
- representsDate
Cypher typeLocalDateTime
- representsLocalDateTime
Cypher typeDateTimeWithZoneOffset
- representsDateTime
Cypher type with time zone as offset in secondsDateTimeWithZoneOffset
- representsDateTime
Cypher type with time zone as IDAll temporal types expose understandable properties. For example
Date
hasyear
,month
andday
properties. Driver uses code adapted from https://github.com/ThreeTen/threetenbp to perform the conversions. Main difference is that driver operates onInteger
objects and not native JavaScript numbers.Temporal types also override
toString()
to return ISO 8601 strings.